Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: whitelist packages on check-licenses action #600

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

RobPasMue
Copy link
Member

Closes #516

Testing done in ansys/pyansys-tools-report#219

@RobPasMue RobPasMue self-assigned this Oct 4, 2024
@RobPasMue RobPasMue requested a review from a team as a code owner October 4, 2024 09:12
@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@github-actions github-actions bot added the enhancement General improvements to existing features label Oct 4, 2024
@@ -182,9 +190,10 @@ runs:
retention-days: 7

- name: Check library's dependencies license
uses: ansys/actions/check-licenses@main
uses: ansys/actions/check-licenses@feat/whitelist-packages
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be reverted to main once approved and before merging

Suggested change
uses: ansys/actions/check-licenses@feat/whitelist-packages
uses: ansys/actions/check-licenses@main

shell: bash
run: |
if [[ -n "${{ inputs.whitelist-license-check }}" ]]; then
echo "Whitelisted packages: ${{ inputs.whitelist-license-check }}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could echo the path of the "ignored-packages.txt" so we can easily access the list.
Also at the top of this file, I will explain why the following packages are for.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content of the ignored-packages.txt file is printed out: see https://github.com/ansys/pyansys-tools-report/actions/runs/11177206820/job/31072221914?pr=219#step:2:829 as an example

The reasons why we are ignoring some of them is already explained in the description I think but we can improve it if you want... what do you have in mind?

See

description: |
Verifies if the licenses of the dependencies installed in the current
environment are compliant with PyAnsys guidelines. This action is assumed to
be used in its own job step. It clones the project and installs the project with
its runtime dependencies.
.. note::
**This action relies on PyPI metadata to identify the license for each package.**
If the metadata are flawed or not included, it may lead to
inconclusive results. In those cases, please perform a thorough review of the
package you are using. Additionally, it is advised not to blindly rely on PyPI metadata.
Even though packages may define their license as of a certain type, the
package could be not applying properly its licensing conditions.
.. jinja:: check-licenses
.. grid:: 1 1 1 2
:gutter: 2
.. grid-item-card:: :octicon:`codescan-checkmark` Accepted third party licenses
{% for license in accepted_licenses %}
* {{ license }}
{% endfor %}
.. grid-item-card:: :octicon:`package` Ignored packages
{% for package in ignored_packages %}
* {{ package }}
{% endfor %}
.. admonition:: Projects requiring additional licenses or packages
If a certain project requires a license or package that is not supported,
`open an issue <https://github.com/ansys/actions/issues>`_ in the
`official ansys/actions repository
<https://github.com/ansys/actions>`_. For additional support, please
contact the `PyAnsys support <mailto:[email protected]>`_.
and the rendering:

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just put the content of the description.
It is just for users that could find the file without knowing about the existence of the action.
Some contributors might see this file without knowing what is its intent because they don't know about all the actions we have.

Copy link
Contributor

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this approach, the one that I had in mind was to allow licenses instead of packages but I think I prefer your approach.
Following @MaxJPRey, it would be great if extra ignored packages could be logged so that you can easily see them if you ever look at the logs. That's a nice way to remember that you ignored packages from the license checker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow users to specify packages to ignore while running the license checker
3 participants